-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: --lockfile-version
to handle string number
#3900
Conversation
As all CLI input is considered to be string, eg. a "npm install --lockfile-version 3" otherwise fails with the error messages: npm WARN invalid config lockfile-version="3" set in command line options npm WARN invalid config Must be one of: null, 1, 2, 3 Replacing `1, 2, 3` with `Number` avoids that and eg. `--lockfile-version 6` will still fail later on with `npm ERR! Invalid lockfileVersion config: 6`, so this is in a way a more DRY approach as well.
--lockfile-version
to handle string number--lockfile-version
to handle string number
Hmm, well this is tricky. We want it to be a number type, but also we want to catch it as a config error rather than a later crash. But you're right, it's not converting the type properly and seeing that it should be numeric. (We cannot get off of nopt soon enough, omg. npm 9 needs to ship like yesterday 😅) I guess in the meantime, until we have a way to set type and possible values, we could take this. Another approach might be to make it |
I tried the |
@voxpelli Ah, yes. Does this do the trick? diff --git a/lib/utils/config/definitions.js b/lib/utils/config/definitions.js
index 3bb8a4210..5dafe0c8d 100644
--- a/lib/utils/config/definitions.js
+++ b/lib/utils/config/definitions.js
@@ -1144,7 +1144,7 @@ define('location', {
define('lockfile-version', {
default: null,
- type: [null, 1, 2, 3],
+ type: [null, 1, 2, 3, '1', '2', '3'],
defaultDescription: `
Version 2 if no lockfile or current lockfile version less than or equal to
2, otherwise maintain current lockfile version
@@ -1166,7 +1166,9 @@ define('lockfile-version', {
on disk than lockfile version 2, but not interoperable with older npm
versions. Ideal if all users are on npm version 7 and higher.
`,
- flatten,
+ flatten: (key, obj, flatOptions) => {
+ flatOptions.lockfileVersion = obj[key] && parseInt(obj[key], 10)
+ },
})
define('loglevel', { |
Closing this in favor of #3949. |
Running "npm install --lockfile-version 3" fails with the error messages:
This is probably because all CLI input is handled as strings and
nopt
strictly comparing those strings to numbers without casting them to numbers. Replacing the[null, 1, 2, 3]
with[null, Number]
makesnopt
type cast and accept all numbers.With this change eg.
--lockfile-version 6
will still fail later on, so it won't open the flood gates for all version numbers. Will still fail with this later in the script:References
Related to #3880